Add a client-side encrypting S3 service - #16
Open
khalilgharbaoui wants to merge 3 commits into
Open
Conversation
Encrypts in the application and hands the bucket ciphertext only, so the storage provider never holds the plaintext - at rest or in transit. It reuses the scheme of the EncryptedDiskService rather than introducing a second one: the schemes only need an IO to read from, and SeekableObjectIO gives them one backed by ranged GET requests, so random access survives the move from a local disk to a bucket.
Skipped unless credentials are in the ENV. Covers what a double cannot: multipart uploads spanning several parts, ranged reads across a part boundary, and an S3-compatible endpoint such as R2.
Author
|
One note on ordering, since these three touch the same gem. If this lands without #15, the gem ends up with two services whose ranged reads disagree: this one converts an exclusive range before handing it to the scheme, while #15 is one line plus a regression test. Taking it first, or both together, avoids the inconsistency. #14 is independent: without it the suite does not boot at all on any Ruby that resolves minitest 6. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fills the gap the README names itself: "this gem does not provide an E2E encrypted solution... you may want to look into S3 client encryption".
ClientSideEncryptedS3Serviceencrypts inside the application process and hands the bucket ciphertext only, so the provider never holds the plaintext, at rest or in transit. WhereEncryptedS3Serviceasks S3 to encrypt for us (SSE-C), this one does not trust the provider with the bytes at all. It also works on S3-compatible providers which have no SSE-C, which is most of them.It adds no new cryptography
That is the part I would most like reviewed, because it is the reason this is small. Your schemes are already IO-agnostic:
V2Schemeneedsread,pos,seekandsize, and does not care whether they come from aFileor from somewhere else. So this PR addsSeekableObjectIO, which serves exactly those four from rangedGETrequests, and a Service shaped likeEncryptedDiskServicewhich hands it to the same scheme, unchanged. Random access survives, which is what makes large media usable.Reads are buffered, or the scheme's small reads (12 bytes of IV, then 16 of tag, then blocks) would each cost a request. The window starts at 64 KB and doubles to 5 MB while reads stay sequential, resetting after a seek, so a one-byte
download_chunkstays cheap while a full download reaches a large window.Decisions worth arguing with
ASEC+ one scheme version byte). An object in a bucket has no filename to carry.encrypted-v2, and asking for object metadata costs a request, so the ciphertext names its own format. It is read as part of the first buffered range, so it is free. The version byte matches your scheme numbering deliberately. Reading an object without the header raisesUnknownCiphertextFormat, which is also how a blob written by a stockS3Serviceannounces itself.private_url_policy: require_headersis refused at construction. A presigned URL can only ever serve ciphertext here, and the client has no key.EncryptedBlobsController, exactly asEncryptedDiskServicedoes, since a browser has no key. No bucket CORS configuration needed.Content-MD5(S3 would check it against our ciphertext). The plaintext is digested as it streams into the cipher, and the object is deleted if they differ.Aws::S3::MultipartUploadErrorwith a dangling multipart upload behind it.Tests
Two layers, because credentials should not decide whether crypto is tested:
test/lib/client_side_encrypted_s3_service_test.rb(21 tests) runs everywhere, against an in-memory bucket built on the SDK's own response stubbing, which honours ranged GETs and multipart. It covers the round trip, ranged reads, wrong keys, checksum failure, compose, and tamper.test/lib/client_side_encrypted_s3_service_against_bucket_test.rbruns against a real bucket when credentials are in the ENV, including an 11 MB upload spanning several parts and ranged reads across a part boundary.S3_ENDPOINTis honoured, so it can be pointed at S3-compatible providers. Verified green against Cloudflare R2.Whole suite: 111 runs, 0 failures.
Honest about what streaming AEAD does not promise
Test-locked rather than asserted in prose:
downloadwith a block yields tampered plaintext and only then raises, because GCM can verify its tag only after the whole ciphertext has been read.downloadwithout a block buffers and raises before returning anything. Both are documented in the class and the README. Fixed-size authenticated frames would close that, and would slot in as a further scheme version rather than a rewrite of any of this - happy to follow up if you like the shape.Also worth saying plainly: this is app-level encryption, not E2E while the app holds the key. The Service takes the key as an argument rather than owning one, so the same code path works whichever way the key is wrapped.